Skip to content

Conversation

@enochterrymize
Copy link

@enochterrymize enochterrymize commented Apr 21, 2021

Ref #20020

@enochterrymize enochterrymize requested a review from skjnldsv April 21, 2021 06:27
import './css/versions.css'
import './versionmodel';
import './versioncollection';
import './versionstabview';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this since you're now providing your own tab.
But for the development phase, you can leave it so you will have 2 versions tab and you will be more at ease when comparing the two :)

Copy link
Author

@enochterrymize enochterrymize Apr 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ll leave it for the development phase

-->

<template>
<li class="version-entry">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a ListItemIcon component

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 143 to 150
this.version.fullPath = fullPath
this.version.fileId = fileId
this.version.name = name
this.version.timestamp = parseInt(moment(new Date(version.timestamp)).format('X'), 10)
this.version.id = OC.basename(version.href)
this.version.size = parseInt(version.size, 10)
this.version.user = user
this.version.client = client
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this.version? It does not seems to be declared anyhere.
I assume there is some code missing or this is still a WIP? 🤔 😉

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed and updated in FileVersion Service

Comment on lines 132 to 137
const fetchVersion = await axios.get(shareUrl, {
params: {
format,
path,
},
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you not using the davClient service(s)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileVersion Service - 6368672

@skjnldsv skjnldsv added 2. developing Work in progress enhancement feature: file sidebar Related to the file sidebar component feature: versions labels Apr 21, 2021
@MorrisJobke MorrisJobke changed the title fix-migrate-tabs-to-vue#20020 Migrate version tab to vue Apr 21, 2021
@MorrisJobke MorrisJobke mentioned this pull request Apr 21, 2021
4 tasks

<template>
<ListItemIcon class="version-entry"
icon="icon-text"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The icon needs to be the mime type.
You can get it with the method: OC.MimeType.getIconUrl(mimetype).
It will return an url of the icon (to be used like the Avatar component: https://github.com/nextcloud/nextcloud-vue/blob/8d1dca2df1e7ee02b50b8b52355f801e6799220a/src/components/ListItemIcon/ListItemIcon.vue#L65)

e.g. OC.MimeType.getIconUrl('image/jpg')

{{ version.timestamp }}Restore
</ActionButton>
<ActionButton icon="icon-delete" @click="alert('Delete')">
Download
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs translation

subtitle="< 1KB">
<Actions>
<ActionButton icon="icon-edit" @click="alert('Edit')">
{{ version.timestamp }}Restore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs translation

if (OCA.Files && OCA.Files.Sidebar) {
OCA.Files.Sidebar.registerTab(new OCA.Files.Sidebar.Tab({
id: 'versions',
name: t('files_versions', 'version'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Versions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

details: true,
}, options))

return response.data.map(FileVersion)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileVersion doesn't exists?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import Videos from '../models/videos'
import Audios from '../models/audios'

export default class Viewer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its removed, it was just for reference

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileInfo: null,
currentUser: null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed and updated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 83 to 85
setCurrentUser(user) {
this._currentUser = user
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed and updated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<IfModule pagespeed_module>
ModPagespeed Off
</IfModule>
#### DO NOT CHANGE ANYTHING ABOVE THIS LINE ####
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change come from your development setup with a specific configuration, so please do not commit that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok updated

const path = (this.fileInfo.path + '/' + this.fileInfo.name).replace('//', '/')
// Fetch Version
const fetchVersion = await axios.get(shareUrl, {
const fetchFileVersions = await axios.get(shareUrl, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be needed as the request would need to be issued by the below call to the webdav client in order to get the list of versions with the following propfind request for example:

image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes removed and Updated

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { translate as t, translatePlural as n } from '@nextcloud/l10n'

import VersionTab from '../../files_versions/src/views/VersionTab'
import TabSections from '../../files_versions/src/services/TabSections'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would this be about? There is not tab section service yet ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log(error);
}
}
mounted() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mounted is not expected to be within the methods but at the root of the vue component object.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok it updated

Copy link
Member

@skjnldsv skjnldsv Apr 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, missing comma on 114

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skjnldsv skjnldsv closed this Jan 10, 2022
@skjnldsv skjnldsv deleted the fix-migrate-tabs-to-vue#20020 branch January 10, 2022 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress enhancement feature: file sidebar Related to the file sidebar component feature: versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants